Skip to content

Conversation

@ilan-theodoro
Copy link

Enabling tensorstore conversion from a zarr array, as mentioned in #26.

@JoOkuma
Copy link
Member

JoOkuma commented Jun 20, 2023

Thanks for the PR, @ilan-francisco.

I have a few minor comments:

  • Could you add a test and document the method?
  • You also need to add the package as a dependency. If it's an optional dependency, pytest can skip the test if the import fails.
  • Convert the assert to an if and raise call.

@ziw-liu , will tensorstore be an optional dependency?

@ziw-liu
Copy link
Contributor

ziw-liu commented Jun 21, 2023

will tensorstore be an optional dependency?

This sounds like the best solution as the compiled extension of tensorstore is not available on every platform.

If we do go this route then this PR (and future ones using ts) will need to handle import errors.

raise NotImplementedError
import tensorstore as ts

assert self.path != "", "Zarr array must be stored on a file system."
Copy link
Contributor

@ziw-liu ziw-liu Jun 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory an iohub NGFF store is always on a local mounted FS, so this might be redundant.

@ziw-liu ziw-liu linked an issue Jun 21, 2023 that may be closed by this pull request
@ziw-liu ziw-liu added enhancement New feature or request NGFF OME-NGFF (OME-Zarr format) performance Speed and memory usage of the code labels Jun 23, 2023
@ziw-liu
Copy link
Contributor

ziw-liu commented Sep 27, 2023

@JoOkuma Is further follow-up work expected for this PR?

@ziw-liu
Copy link
Contributor

ziw-liu commented Nov 8, 2023

Closing as stale.

@ziw-liu ziw-liu closed this Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request NGFF OME-NGFF (OME-Zarr format) performance Speed and memory usage of the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tensorstore support

3 participants